feat(zcoin): allow ARRR to sync using a start date by borngraced · Pull Request #1922 · GLEECBTC/komodo-defi-framework

borngraced · 2023-07-19T09:08:19Z

Improve ARRR synchronization based on a user-selected date. This feature will enable users to specify a specific date as the starting point for synchronization as a substitute for the checkpoint block from config or syncing from the first block.

Tasks

Added LightWalletSyncParams.date, 'earliestandheight` param for synchronising blocks based on user-selected date, earliest or height for light client

#1900

…s optimization Signed-off-by: borngraced <samuelonoja970@gmail.com>

Signed-off-by: borngraced <samuelonoja970@gmail.com>

shamardy · 2023-07-20T15:33:12Z

@borngraced please fix wasm clippy warnings and merge to latest dev to pass the adex-cli tests.

shamardy

Thanks a lot for the fast PR @borngraced! First review iteration, I believe there will be 2 more review iterations after this :)

Signed-off-by: borngraced <samuelonoja970@gmail.com>

… calculate_starting_height_from_date and WalletDbShared init Signed-off-by: borngraced <samuelonoja970@gmail.com>

Signed-off-by: borngraced <samuelonoja970@gmail.com>

shamardy

Thanks for the fixes! Next review iteration!

Can you please add a test to test zcoin initialization using date, height or default behaviour. All zcoin tests using light client are ignored, you can try default behaviour for this test and remove the ignore flag https://github.com/KomodoPlatform/komodo-defi-framework/blob/a1fc8f7c92819f68c0dcdf7b1c650d86cdf945ad/mm2src/mm2_main/tests/mm2_tests/z_coin_tests.rs#L473 No need to use date or height, you just need to remove checkpoint block from pirate configuration

shamardy · 2023-07-26T02:30:12Z

+                    },
+                }
+            },
+            ZcoinRpcMode::Native => zcoin_builder.protocol_info.check_point_block.clone(),


Native should be the same as light client as discussed.

This can be done in another PR as we agreed. Please don't forget to open an issue for it.

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced · 2023-08-25T10:33:51Z

Ran a couple hundred activation sequence variations in CLI, no problems with activation using expected inputs. There are a few cases where it ignores bad input, not sure if this is a concern but just in case:

Handles more than one sync_params value (unsure of hierarchy, but no error)

No error on negative value for date, starts from beginning

'sync_params': 'bad_key' does not raise an error, scans from beginning

Yes, @naezith and I concluded on using the earliest date incase bad date/height was provided

smk762

LGTM, thanks!

shamardy

@Alrighttt @ca333 @DeckerSU To help with security review, I marked the places where I think it's required. You can ofcourse check the whole PR for other things :)

shamardy · 2023-08-29T05:34:51Z

+    pub(crate) async fn get_earliest_block(&self) -> Result<u32, ZcashClientError> {
+        Ok(query_single_row(
+            &self.db.lock().unwrap(),
+            "SELECT MIN(height) from compactblocks",
+            [],
+            |row| row.get::<_, Option<u32>>(0),
+        )?
+        .flatten()
+        .unwrap_or(0))
+    }


shamardy · 2023-08-29T05:35:02Z

+        run: |
+          Invoke-WebRequest -Uri https://github.com/KomodoPlatform/komodo/raw/d456be35acd1f8584e1e4f971aea27bd0644d5c5/zcutil/wget64.exe -OutFile \wget64.exe
+          Invoke-WebRequest -Uri https://raw.githubusercontent.com/KomodoPlatform/komodo/master/zcutil/fetch-params-alt.bat -OutFile \cmd.bat && \cmd.bat
+          cargo test --test 'mm2_tests_main' --no-fail-fast


shamardy · 2023-08-29T05:36:47Z

+        run: |
+          wget -O - https://raw.githubusercontent.com/KomodoPlatform/komodo/master/zcutil/fetch-params-alt.sh | bash
+          cargo test --test 'mm2_tests_main' --no-fail-fast


Signed-off-by: borngraced <samuelonoja970@gmail.com>

ca333

secure code reviewed

shamardy · 2023-09-04T05:35:44Z

@borngraced can you please resolve the git conflicts in this PR?

# Conflicts: # mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs # mm2src/coins/z_coin.rs # mm2src/coins/z_coin/storage/walletdb/mod.rs # mm2src/mm2_main/tests/integration_tests_common/mod.rs # mm2src/mm2_main/tests/mm2_tests/z_coin_tests.rs # mm2src/mm2_test_helpers/src/for_tests.rs

Signed-off-by: borngraced <samuelonoja970@gmail.com>

laruh

Good job!
As we are going to prepare our code base for missing_docs lint in CI, its necessary to add doc comments for all pub items, bcz lint will deny pub items without them.
For the current PR please do it at least for items, which were added or changed.

Signed-off-by: borngraced <samuelonoja970@gmail.com>

laruh

Thank you for your work!
I have just a note for the future. Perhaps you have already discussed this in conversations above, if so, what decision did you make?

laruh · 2023-09-07T05:19:35Z


+    /// Asynchronously retrieve the tree state for a specific block height from the Zcoin network.
+    #[cfg(not(target_arch = "wasm32"))]
+    async fn get_tree_state(&mut self, height: u64) -> Result<TreeState, MmError<UpdateBlocksCacheErr>>;


NativeClient doesnt support grpc client (i mean you return TreeState here, which is protobuf message), but as get_tree_state is in todo (in NativeClient) and we are going to use this functionality on LightRpcClient for now, I think its fine.

plan to implement a get_tree_state, LightdInfo rpc methods for native client.

save dev state — impl LightWalletSyncParams for block synchronization…

e7b459b

…s optimization Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced self-assigned this Jul 19, 2023

save dev state — add docs and minor changes

eefe9d4

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced added the in progress label Jul 19, 2023

borngraced marked this pull request as draft July 19, 2023 09:31

borngraced added 3 commits July 19, 2023 15:42

save dev state — more changes to get_tree_state and coin params

7585df2

Signed-off-by: borngraced <samuelonoja970@gmail.com>

save dev state — minor changes

f260b7a

Signed-off-by: borngraced <samuelonoja970@gmail.com>

save dev state — minor changes to fnand variable namings

56621c7

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced marked this pull request as ready for review July 19, 2023 17:42

borngraced changed the title ~~Implement ARRR Sync Starting Block Selection(Date and height)~~ feat(zcoin): implement ARRR Sync Starting Block Selection(Date and height) Jul 19, 2023

borngraced requested review from artemii235, ca333 and shamardy July 20, 2023 02:41

borngraced added the under review label Jul 20, 2023

shamardy removed the in progress label Jul 20, 2023

shamardy changed the title ~~feat(zcoin): implement ARRR Sync Starting Block Selection(Date and height)~~ feat(zcoin): allow ARRR to sync using a start date Jul 20, 2023

shamardy requested changes Jul 20, 2023

View reviewed changes

borngraced added 4 commits July 21, 2023 17:20

fix review notes

aabd924

Signed-off-by: borngraced <samuelonoja970@gmail.com>

fix import error

0de4fbf

Signed-off-by: borngraced <samuelonoja970@gmail.com>

use appropriate error handling instead of unwrap and minor changes to…

5e4ab78

… calculate_starting_height_from_date and WalletDbShared init Signed-off-by: borngraced <samuelonoja970@gmail.com>

rename buffer -> avg_blocktime

8e9f78a

Signed-off-by: borngraced <samuelonoja970@gmail.com>

shamardy added P1 labels Jul 26, 2023

shamardy requested changes Jul 26, 2023

View reviewed changes

shamardy added in progress and removed under review labels Jul 26, 2023

borngraced added 3 commits July 26, 2023 22:09

fix review notes — refactoring

3693272

Signed-off-by: borngraced <samuelonoja970@gmail.com>

remove get_minimum_header_from_cache

6b33b54

Signed-off-by: borngraced <samuelonoja970@gmail.com>

Merge branch 'dev' into zcoin_light_client_opt

c4b0426

smk762 previously approved these changes Aug 25, 2023

View reviewed changes

shamardy previously approved these changes Aug 29, 2023

View reviewed changes

add alias for electrum_servers

de50159

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced dismissed stale reviews from shamardy and smk762 via de50159 August 29, 2023 11:11

smk762 mentioned this pull request Aug 31, 2023

Removes obsolete zhtlc fields, adds zhtlc derivation paths GLEECBTC/coins#809

Closed

ca333 previously approved these changes Sep 1, 2023

View reviewed changes

borngraced added 2 commits September 4, 2023 11:43

fix merge conflicts

ca0dbdc

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced dismissed ca333’s stale review via ca0dbdc September 4, 2023 11:20

laruh suggested changes Sep 5, 2023

View reviewed changes

laruh mentioned this pull request Sep 5, 2023

feat(zcoin): ARRR WASM implementation #1957

Merged

8 tasks

add doc comments

04b8f6c

Signed-off-by: borngraced <samuelonoja970@gmail.com>

smk762 requested review from kivqa and smk762 September 6, 2023 10:53

smk762 previously approved these changes Sep 6, 2023

View reviewed changes

add remaining doc comments

1b7c689

Signed-off-by: borngraced <samuelonoja970@gmail.com>

borngraced dismissed smk762’s stale review via 1b7c689 September 6, 2023 13:45

laruh approved these changes Sep 7, 2023

View reviewed changes

shamardy approved these changes Sep 7, 2023

View reviewed changes

shamardy merged commit 1538564 into dev Sep 7, 2023

shamardy deleted the zcoin_light_client_opt branch September 7, 2023 09:00

shamardy mentioned this pull request Sep 7, 2023

chore(release): v1.0.7-beta #1936

Merged

This was referenced Sep 11, 2023

[BUG]: ZHTLC sync progress GLEECBTC/gleec-wallet#2354

Closed

Upgrade Z coin activation GLEECBTC/gleec-wallet#2355

Closed

Zcoin activation date GLEECBTC/gleec-wallet#2356

Merged

shamardy mentioned this pull request Jan 15, 2024

Add start date parameter to zcoin activation request #1900

Closed

smk762 removed the docs label Nov 1, 2024

Conversation

borngraced commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shamardy commented Jul 20, 2023

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shamardy Jul 26, 2023

Choose a reason for hiding this comment

Uh oh!

shamardy Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

borngraced commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smk762 left a comment

Choose a reason for hiding this comment

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

shamardy Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

shamardy Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

shamardy Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

ca333 left a comment

Choose a reason for hiding this comment

Uh oh!

shamardy commented Sep 4, 2023

Uh oh!

laruh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laruh left a comment

Choose a reason for hiding this comment

Uh oh!

laruh Sep 7, 2023

Choose a reason for hiding this comment

Uh oh!

borngraced Sep 7, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

borngraced commented Jul 19, 2023 •

edited

Loading

borngraced commented Aug 25, 2023 •

edited

Loading